-
Notifications
You must be signed in to change notification settings - Fork 213
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Error and SetError to all datasets. #152
Conversation
This addresses #150 by adding SetError and Error methods to each dataset. This will allow end-user query building (e.g. users of goqu) to be able to mark a dataset as being in a failed state via SetError and not have to track that separately. It only sets the error if one has not already been set. It also sets it on the underlying SQLBuilder so that it too will know about the error and not try to proceed with any futher query building. As such, future calls to ToSQL will return the error as well.
Quick turnaround! My only ask is to add some docs around this and why one might use it, as I think quite a few users of this library may find this useful as it could really streamline some code that is focused on dynamically building queries. Great Job! |
I'll add some documentation around this tonight... Thanks! |
…nd Error and ToSQL.
Hey @doug-martin check out the latest commit and see what you think. I added some documentation and examples to each dataset's docs. Let me know if you want something changed or explained differently. |
var ds = goqu.Delete("test"). | ||
Where(goqu.C(name).Eq(value)) | ||
|
||
if len(name) == 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this is pedantic but I think it makes sense to put the validation before setting the where. In the end it wont make a difference based on the implementation; but setting the error before creating the clause signifies intention.
I often struggle with showing intention vs how its actually implemented, but in this case validation should come first.
If you disagree let me know, but I want docs to demonstrate how it should be used and not necessarily how it could be used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I totally agree. I didn't spend a ton of time thinking about the examples from that perspective but you're totally right. I'll push an update. Let me know if you find other things that are confusing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for being so receptive to comments! I think the same applies for the rest of the examples (they all seem to be based off of this example). Should be a quick update that will helps many users in the long run.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once you update Ill merge update history and release!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absolutely! Yes, I copy-pasted this example to the others and made edits where necessary.
I'll fix the other examples too.
This addresses #150 by
adding SetError and Error methods to each dataset. This will allow
end-user query building (e.g. users of goqu) to be able to mark a
dataset as being in a failed state via SetError and not have to track
that separately.
It only sets the error if one has not already been set. It also sets it
on the underlying SQLBuilder so that it too will know about the error
and not try to proceed with any futher query building. As such, future
calls to ToSQL will return the error as well.